feat(analytics-controller): add optional invocation options for track, identify, view#8701
feat(analytics-controller): add optional invocation options for track, identify, view#8701gauthierpetetin wants to merge 5 commits into
Conversation
…, identify, view Forward AnalyticsInvocationOptions (context, callback, messageId, timestamp) from AnalyticsController to AnalyticsPlatformAdapter for trackEvent, identify, and trackView. Extend adapter method signatures with optional third argument. Export AnalyticsContext and AnalyticsInvocationCallback. Co-authored-by: Cursor <cursoragent@cursor.com>
gauthierpetetin
force-pushed
the
feat/analytics-invocation-options
branch
from
7a3f783 to
002a571
Compare
gauthierpetetin
temporarily deployed
to
default-branch
GitHub Actions
Inactive
Co-authored-by: Cursor <cursoragent@cursor.com>
| /** | ||
| * Optional context object attached to the invocation. | ||
| */ | ||
| context?: AnalyticsContext; |
There was a problem hiding this comment.
Q: there's already a context attached by Segment SDK, how does this interfere with it?
There was a problem hiding this comment.
Extension adds context to events, that Segment SDK doesn't provide (cf. this code).
Segment can still add its own SDK context. We are not replacing that.
We are allowing the adapter to pass additional context through to Segment. If there are key collisions, that is a platform-adapter concern, so Extension should avoid overriding SDK-owned fields unless intentional.
So far, the only SDK-owned fields are:
context.library.name = '@segment/analytics-node'
context.library.version = <package version>
There was a problem hiding this comment.
Will all these values be in the same context object in the final event payload?
| /** | ||
| * Optional callback when the invocation completes or fails. | ||
| */ | ||
| callback?: AnalyticsInvocationCallback; |
There was a problem hiding this comment.
Q: What are the use cases of this? The tracking should never block or interfere with UX, so it's best effort and errors are handled inside controller, right?
There was a problem hiding this comment.
See message here.
Also callback is not for UX and should not be awaited by product flows. It is only the cleanup signal: once Segment finishes processing the call, we remove that messageId from the persisted queue.
| /** | ||
| * Optional stable identifier for deduplication or tracing. | ||
| */ | ||
| messageId?: string; |
There was a problem hiding this comment.
Q; we already have a message ID in the Segment payload. How is this different?
There was a problem hiding this comment.
See message here.
We need a messageId that is defined by MetaMetricsController before sending the event to Segment, in order to persist the event queue.
| /** | ||
| * Optional event time. ISO strings, Unix timestamps in milliseconds, or Date values are typical. | ||
| */ | ||
| timestamp?: Date | number | string; |
There was a problem hiding this comment.
Q: we already have a timestamp in Segment payload. How is this different?
There was a problem hiding this comment.
We need a timestamp that is defined by MetaMetricsController before sending the event to Segment, in order to preserve the original event time when the event is replayed later, thanks to the persisted event queue.
|
@NicolasMassart the reason why we need to pass On Mobile, that’s achieved thanks to Segment SDK’s That's quite problematic by the way, because it prevents us from fully getting rid of the existing MetaMetricController: we still need it besides AnalyticsController in order to persist the event queue. |
|
Only comment is if we could explore the Segment SDK plugin system more to see if we can have these event data added in a plugin to prevent changes in the event on the app side. |
2 participants
Explanation
Add the possibility to forward
context,callback,messageId, andtimestampto AnalyticsController for trackEvent, identify, and trackView functions, , as this is needed in Extension.References
NA
Checklist
Note
Medium Risk
Updates the public
AnalyticsControllerandAnalyticsPlatformAdaptermethod signatures to accept new optional invocation metadata, which may require downstream adapter/consumer updates. Adds special handling formessageIdwhen emitting split anonymous/sensitive events, which could affect event deduplication behavior.Overview
Adds
AnalyticsInvocationOptions(e.g.,context,callback,messageId,timestamp) and threads it throughAnalyticsController.trackEvent,identify, andtrackViewintoAnalyticsPlatformAdapter.track/identify/view.When anonymous-event splitting is enabled and a
messageIdis provided, the controller now derives a distinctmessageIdfor the anonymous/sensitive counterpart to avoid SDK deduping. Tests, public exports (index.ts), generated method action types, and the package changelog are updated accordingly.Reviewed by Cursor Bugbot for commit 94fc58c. Bugbot is set up for automated code reviews on this repo. Configure here.